Remove pathlib.PurePath.__eq__#10662
Conversation
This comment has been minimized.
This comment has been minimized.
|
I think the primer hits here are real issues |
hauntsaninja
force-pushed
the
pathlib-strict-eq
branch
from
659aa6c to
7cf01aa
Compare
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Alex Waygood <[email protected]>
There was a problem hiding this comment.
It still feels like we're sorta working around shortcomings in the heuristics mypy/pyright use here. (As you said in the issue, this kind of thing is going to affect anybody who writes a __eq__ method in a py.typed runtime library, and __eq__ methods are pretty common!)
But I agree that this is a pretty useful error for mypy/pyright to emit, so I'm happy for this change to be made on the basis that practicality beats purity :-)
This comment has been minimized.
This comment has been minimized.
|
Sounds good, I'll leave open for a bit in case anyone else chimes in! |
|
Here is a small regression that this PR causes, FYI: currently mypy will not complain about this code (on my Windows machine, the expression evaluates to from pathlib import *
PureWindowsPath("foo/bar") == Path("foo/bar")But with this PR, it will: $ mypy -c "from pathlib import *; PureWindowsPath('foo/bar') == Path('foo/bar')" --strict-equality --custom-typeshed-dir .
<string>:1: error: Non-overlapping equality check (left operand type: "PureWindowsPath", right operand type: "Path") [comparison-overlap]
Found 1 error in 1 file (checked 1 source file)Is the regression that important? Probably not particularly; I don't know how many people are doing this kind of comparison. But, worth noting, I think. |
|
@AlexWaygood oh, interesting example! Definitely a false positive in terms of what the type checker thinks it's doing, but that code is pretty suspect and I'm okay with it triggering a warning of some kind — depending on your platform, it could be true that those could never be equal! So I think I'm still fine with the PR (though I'll certainly add test case and comments). We should experiment with making |
|
Diff from mypy_primer, showing the effect of this PR on open source code: mkosi (https://github.com/systemd/mkosi)
+ mkosi/kmod.py:32:16: error: Non-overlapping container check (element type: "str", container item type: "Path") [comparison-overlap]
+ mkosi/config.py:2012:8: error: Non-overlapping equality check (left operand type: "Optional[Path]", right operand type: "Literal['']") [comparison-overlap]
|
|
This change caught a real bug in stubtest (that was introduced by, er, me)! https://github.com/python/mypy/actions/runs/6202155467/job/16840319289?pr=16121 The first CI run on python/mypy#16121 |
3 participants
Fixes #10661